Skip to content

[RFC]: Add file_descriptor() function to retrieve the file descriptor of a PHP Stream #10342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jan 16, 2023

RFC: https://wiki.php.net/rfc/file-descriptor-function

The purpose of this function is to retrieve the underlying file descriptors for PHP streams when they exist.
It is currently possible to achieve this result by using FFI and stubbing the Zend engine (see: https://github.com/ppelisset/php-fileno).
This can be needed when trying to interact with a USB device, as this is the use case @ppelisset has.

Test should be based of #10173 when it gets merged.

* It is only used here so that the buffered data warning is not displayed.
*/
if (php_stream_can_cast(stream, PHP_STREAM_AS_FD | PHP_STREAM_CAST_INTERNAL) == FAILURE) {
zend_argument_type_error(1, "cannot represent as a file descriptor");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
zend_argument_type_error(1, "cannot represent as a file descriptor");
zend_argument_type_error(1, "cannot be represented as a file descriptor");

@azjezz
Copy link

azjezz commented Feb 1, 2023

it think we miss a test case for closed resources, i.e:

$fp = fopen('somefile.txt', 'wb+');
fclose($fp);

try {
  var_dump(file_descriptor($fp));
} catch ( ... ) {
  ...
}


php_stream_from_zval(stream, zsrc);

/* TODO Should support streams that can be cast with PHP_STREAM_AS_FD_FOR_SELECT ? */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general PHP_STREAM_AS_FD should be supported if PHP_STREAM_AS_FD_FOR_SELECT which should be the case for all core stream wrappers - I just checked and don't see a case where it would be different. So I wouldn't bother with checking PHP_STREAM_AS_FD_FOR_SELECT as well.

Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good in general. The test failure seems related as you probably know.

@Girgias Girgias requested a review from kocsismate as a code owner April 15, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants